-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added several plan building speed improvements #5
Conversation
@@ -30,6 +30,7 @@ def install_with_constraints(session, *args, **kwargs): | |||
"poetry", | |||
"export", | |||
"--dev", | |||
"--without-hashes", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the correct action based on python-poetry/poetry#3472 (search "without-hashes")
@@ -1,258 +1,250 @@ | |||
[[package]] | |||
category = "dev" | |||
description = "A configurable sidebar-enabled Sphinx theme" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seemingly just bumps a bunch of versions...
Codecov Report
@@ Coverage Diff @@
## main #5 +/- ##
==========================================
+ Coverage 92.77% 92.82% +0.05%
==========================================
Files 44 44
Lines 1619 1631 +12
Branches 230 226 -4
==========================================
+ Hits 1502 1514 +12
Misses 94 94
Partials 23 23
Continue to review full report at Codecov.
|
@@ -88,24 +92,24 @@ def prep_run_physical( | |||
plan, output_node | |||
) | |||
plan = prune_source_literals(plan, inplace=inplace) | |||
on_started = on_started or identity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This avoids three null checks for each node...
raise | ||
finally: | ||
bound_call.value = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving this to the finally block fixes a memory leak related to failures. In practice this probably didn't matter.
@@ -55,7 +55,7 @@ def _create_bound_call_lookup_and_output_slot( | |||
plan: Plan, output_node: Optional[Node] = None | |||
): | |||
result_lookup = { | |||
node: Slot(node.value if type(node) is Literal else None) | |||
node: node if type(node) is Literal else Slot(None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tiny step towards eliminating Slot from run_physical entirely, which is possible but I don't want to do as part of this PR.
@@ -32,6 +32,7 @@ isort = "^5.5.3" | |||
pytest = "^5.2" | |||
pytest-cov = "^2.10.1" | |||
releases = "^1.6.3" | |||
six = "^1.15.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This addresses an issue with CI.
No description provided.